Offense Stat Line Limitations - #306

Maple2.Server.Game/Util/ItemStatsCalculator.cs (1)

472-503: Prevent potential infinite loop when no valid stat can be added.
Because the code continues looping until statResult.Count + specialResult.Count == total, but skips offense stats when IsValidStat fails, there's a risk of never exiting if all candidates are invalid.

Consider adding a maximum iteration safeguard. For example:

 while (statResult.Count + specialResult.Count < total) {
+    const int maxAttempts = 1000;
+    int attempts = 0;
+
+    // ...
+    while (statResult.Count + specialResult.Count < total && attempts < maxAttempts) {
+        attempts++;
         ItemOption.Entry entry = option.Entries.Random();
         if (statsType == ItemStats.Type.Random
             && itemType.IsArmor
             && !IsValidStat(total, statResult, specialResult, entry)) {
             continue;
         }
         // ...
+    }
+    if (attempts >= maxAttempts) {
+       Log.Warning("Reached maximum iteration attempts while filling item stats.");
+       break;
+    }
 }

This ensures the code eventually exits, even if no valid offense attributes remain.

Maple2.Server.Game/Util/ItemStatsCalculator.cs (2)

365-373: Consider renaming or refactoring for clarity.
Invoking “RandomItemOption” from “GetStaticOption” could be confusing for future maintainers, as “RandomItemOption” is also used for truly random scenarios. A more neutral helper name (e.g., “GenerateItemOption”) could reduce confusion.

---

394-396: Simplify or remove the pass-through function.
GetRandomOption immediately delegates to RandomItemOption, adding a minor layer of indirection. If no extra logic is planned, consider removing it in favor of calling RandomItemOption directly.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Maple2.Server.Game/Util/ItemStatsCalculator.cs (1)

556-583: Optimize the validation logic with early returns.

The method can be simplified for better readability:

 private static bool IsValidStat(ItemType itemType, int statLineCount, IDictionary<BasicAttribute, BasicOption> statDict, IDictionary<SpecialAttribute, SpecialOption> specialDict, ItemOption.Entry entry) {
-    if (itemType.IsAccessory) {
-        int currentElementalStats = specialDict.Keys.Count(stat => elementalDamageAttributes.Contains(stat));
-        if (entry.SpecialAttribute != null && elementalDamageAttributes.Contains((SpecialAttribute) entry.SpecialAttribute)) {
-            currentElementalStats++;
-        }
-        return currentElementalStats < 2;
-    }
+    // Early return for non-armor/accessory items
+    if (!itemType.IsArmor && !itemType.IsAccessory) {
+        return true;
+    }
 
-    if (itemType.IsArmor) {
-        int offenseStatCount = statDict.Keys.Count(stat => offenseBasicAttributes.Contains(stat));
-        offenseStatCount += specialDict.Keys.Count(stat => offenseSpecialAttributes.Contains(stat));
+    // Handle accessories
+    if (itemType.IsAccessory) {
+        int currentElementalStats = specialDict.Keys.Count(stat => elementalDamageAttributes.Contains(stat));
+        return !(entry.SpecialAttribute != null && 
+                elementalDamageAttributes.Contains((SpecialAttribute)entry.SpecialAttribute) && 
+                currentElementalStats >= 1);
+    }
 
-        if (entry.BasicAttribute != null && offenseBasicAttributes.Contains((BasicAttribute) entry.BasicAttribute)) {
-            offenseStatCount++;
-        } else if (entry.SpecialAttribute != null && offenseSpecialAttributes.Contains((SpecialAttribute) entry.SpecialAttribute)) {
-            offenseStatCount++;
-        }
+    // Handle armor
+    int offenseStatCount = statDict.Keys.Count(stat => offenseBasicAttributes.Contains(stat)) +
+                          specialDict.Keys.Count(stat => offenseSpecialAttributes.Contains(stat));
 
-        return (float) offenseStatCount / statLineCount <= OFFENSE_LINE_MAX_THRESHOLD;
+    if (entry.BasicAttribute != null && offenseBasicAttributes.Contains((BasicAttribute)entry.BasicAttribute)) {
+        offenseStatCount++;
+    } else if (entry.SpecialAttribute != null && offenseSpecialAttributes.Contains((SpecialAttribute)entry.SpecialAttribute)) {
+        offenseStatCount++;
     }
 
-    return true;
+    return (float)offenseStatCount / statLineCount <= OFFENSE_LINE_MAX_THRESHOLD;
 }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Maple2.Server.Game/Util/ItemStatsCalculator.cs (2)

416-418: Consider reordering parameters for better readability.

The parameter order could be improved by grouping related parameters together. Consider moving count next to presets since they're both optional parameters.

-private ItemStats.Option GetRandomOption(ItemOption itemOption, in ItemType itemType, ItemStats.Type statsType, int count = -1, params LockOption[] presets)
+private ItemStats.Option GetRandomOption(ItemOption itemOption, in ItemType itemType, ItemStats.Type statsType, params LockOption[] presets, int count = -1)

---

565-590: Improve method documentation and readability.

1. Add parameter documentation to explain the purpose of each parameter.
2. Consider extracting the offense stat count calculation into a separate method for better readability.

     /// <summary>
     /// Verifies if the new attribute being added meets the offense line threshold.
     /// </summary>
+    /// <param name="itemType">The type of item being validated</param>
+    /// <param name="statLineCount">Total number of stat lines</param>
+    /// <param name="statDict">Dictionary of basic attributes</param>
+    /// <param name="specialDict">Dictionary of special attributes</param>
+    /// <param name="entry">The new entry being validated</param>
+    /// <returns>True if the stat is valid according to offense line limitations</returns>
     private static bool IsValidStat(ItemType itemType, int statLineCount, IDictionary<BasicAttribute, BasicOption> statDict, IDictionary<SpecialAttribute, SpecialOption> specialDict, ItemOption.Entry entry) {
         int damageTypeStatCount = specialDict.Keys.Count(stat => damageTypeAttributes.Contains(stat));
         if (entry.SpecialAttribute != null && damageTypeAttributes.Contains((SpecialAttribute) entry.SpecialAttribute)) {
             return damageTypeStatCount < 1;
         }

         if (!itemType.IsCombatPet && !itemType.IsWeapon && !itemType.IsAccessory) {
-            int offenseStatCount = statDict.Keys.Count(stat => offenseBasicAttributes.Contains(stat));
-            offenseStatCount += specialDict.Keys.Count(stat => offenseSpecialAttributes.Contains(stat));
-
-            if (entry.BasicAttribute != null && offenseBasicAttributes.Contains((BasicAttribute) entry.BasicAttribute)) {
-                offenseStatCount++;
-            } else if (entry.SpecialAttribute != null && offenseSpecialAttributes.Contains((SpecialAttribute) entry.SpecialAttribute)) {
-                offenseStatCount++;
-            }
+            int offenseStatCount = CalculateOffenseStatCount(statDict, specialDict, entry);
+            return (float)offenseStatCount / statLineCount <= OFFENSE_LINE_MAX_THRESHOLD;
         }

         return true;
     }

+    private static int CalculateOffenseStatCount(
+        IDictionary<BasicAttribute, BasicOption> statDict,
+        IDictionary<SpecialAttribute, SpecialOption> specialDict,
+        ItemOption.Entry entry)
+    {
+        int count = statDict.Keys.Count(stat => offenseBasicAttributes.Contains(stat));
+        count += specialDict.Keys.Count(stat => offenseSpecialAttributes.Contains(stat));
+
+        if (entry.BasicAttribute != null && offenseBasicAttributes.Contains((BasicAttribute)entry.BasicAttribute))
+        {
+            count++;
+        }
+        else if (entry.SpecialAttribute != null && offenseSpecialAttributes.Contains((SpecialAttribute)entry.SpecialAttribute))
+        {
+            count++;
+        }
+
+        return count;
+    }

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

Maple2.Server.Game/Util/ItemStatsCalculator.cs (2)

22-22: Consider making the threshold configurable.

The offense line threshold is currently hardcoded. Consider making it configurable through game settings or configuration files for easier tuning.

---

571-574: Document the damage type attribute limitation.

Add a comment explaining why only one damage type attribute is allowed per item. This will help future maintainers understand the game design decision.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro